[SPARK-57555][FOLLOWUP][SQL] Add TimeType support to MySQLDialect by shrirangmhalgi · Pull Request #57198 · apache/spark

shrirangmhalgi · 2026-07-11T06:30:59Z

What changes were proposed in this pull request?

Add TimeType support to MySQLDialect and precision-preserving DDL for writes:

getCatalystType: map Types.TIME to TimeType(scale) when spark.sql.timeType.enabled is true, gated by legacyJdbcTimeMappingEnabled
getJDBCType: add TimeType => TIME(p) for p in [0,6], bare TIME for out-of-range precisions (e.g. nanosecond TimeType)

Why are the changes needed?

This is a followup to #56653 which added core JDBC TIME support in JdbcUtils. That PR handled scalar read/write correctly via the generic path, but:

The write-side getJDBCType in the base JdbcUtils emits TIME(${t.precision}) which produces TIME(9) for nanosecond TimeTypes - MySQL rejects precisions > 6
The per-dialect getCatalystType runs before the generic JdbcUtils mapper (confirmed in [SPARK-57555][FOLLOWUP][SQL] Add escape-hatch conf to keep legacy JDBC TIME-to-timestamp mapping #56884), so MySQL needs its own TIME handling gated by the escape-hatch config

Does this PR introduce any user-facing change?

Yes. When spark.sql.timeType.enabled is true, MySQL TIME and TIME(p) columns now correctly read as TimeType and write with precision-preserving TIME(p) DDL. Non-TIME columns are unaffected.

How was this patch tested?

Added integration tests in MySQLIntegrationSuite:

Scalar read: verifies existing dates table TIME column is read as TimeType(0) and TIME(3) as TimeType(3) with correct values
Scalar write round-trip: writes TimeType(0) and TimeType(6) values and reads them back
Precision preservation: TimeType(3) -> TIME(3) -> read back as TimeType(3)
Nanosecond write: TimeType(9) -> bare TIME -> read back as TimeType(6) with microsecond truncation
Legacy mode: asserts TIME reads as non-TimeType when legacyJdbcTimeMappingEnabled=true

Unit tests: JDBCSuite (127 tests, all passing).

Was this patch authored or co-authored using generative AI tooling?

CoAuthored using Claude Opus 4.6

MaxGekk

2 blocking, 1 non-blocking, 0 nits.
The read path is correct and well-tested, but the write path silently loses all fractional seconds for nanosecond TimeType. The one test that would catch it asserts the correct behavior yet fails against the current code on real MySQL — and isn't run in standard CI.

Correctness (2)

MySQLDialect.scala:333: out-of-range fallback emits bare TIME = MySQL TIME(0), losing all fractional seconds for TimeType(7/8/9); should emit TIME(6) — see inline
MySQLIntegrationSuite.scala:435: nanosecond write test asserts the correct TimeType(6)/microsecond round-trip, which the current TIME(0) fallback can't satisfy — it would fail on real MySQL, and it's a @DockerTest not run in standard CI — see inline

Suggestions (1)

MySQLDialect.scala:228: getLong("datetimePrecision") throws if the key is absent; currently safe but fragile — see inline

Verification

Traced read/write symmetry against MySQL's bare-TIME = TIME(0) semantics. The PR's own read test (inserts 13:31:24.123 into a bare TIME column, asserts read-back 13:31:24 / TimeType(0)) confirms bare TIME drops fractional seconds — so the write else-branch's bare TIME for precision>6 loses all sub-second data rather than truncating to micros. The updateExtraColumnMeta→getCatalystType ordering in JdbcUtils.getSchema (call at line 331 before line 334) makes the read path's metadata read safe.

MaxGekk · 2026-07-14T07:49:11Z

+      // MySQL supports TIME(p) for p in [0,6]. Use the declared precision when valid;
+      // fall back to bare TIME (= TIME(6)) for out-of-range precisions (e.g. nanosecond).
+      val p = t.precision
+      if (p >= 0 && p <= 6) Option(JdbcType(s"TIME($p)", java.sql.Types.TIME))
+      else Option(JdbcType("TIME", java.sql.Types.TIME))


This out-of-range fallback loses all fractional seconds, not just the sub-microsecond part. TimeType.MAX_PRECISION is 9, so TimeType(7|8|9) reaches the else and emits bare TIME — which in MySQL is TIME(0) (whole seconds), as this PR's own read test confirms (inserting 13:31:24.123 into a bare TIME column reads back 13:31:24). So 12:00:00.123456789 is stored as 12:00:00, not truncated to microseconds. MySQL's max fractional precision is 6, so the fallback should emit TIME(6):

Suggested change

// MySQL supports TIME(p) for p in [0,6]. Use the declared precision when valid;

// fall back to bare TIME (= TIME(6)) for out-of-range precisions (e.g. nanosecond).

val p = t.precision

if (p >= 0 && p <= 6) Option(JdbcType(s"TIME($p)", java.sql.Types.TIME))

else Option(JdbcType("TIME", java.sql.Types.TIME))

// MySQL supports TIME(p) for p in [0,6]. Use the declared precision when valid;

// fall back to TIME(6) (MySQL's max fractional precision) for out-of-range

// precisions (e.g. nanosecond), which truncates to microseconds rather than

// to whole seconds (bare TIME = TIME(0) in MySQL).

val p = t.precision

if (p >= 0 && p <= 6) Option(JdbcType(s"TIME($p)", java.sql.Types.TIME))

else Option(JdbcType("TIME(6)", java.sql.Types.TIME))

MaxGekk · 2026-07-14T07:49:11Z

+      assert(result.schema("t_nanos").dataType === TimeType(6))
+      // Nanoseconds truncated to microseconds: 123456789 -> 123456000
+      checkAnswer(result, Row(LocalTime.of(12, 0, 0, 123456000)))


These assertions are correct — TimeType(6) with microseconds preserved is what a nanosecond write should round-trip to. But they don't hold against the current code: getJDBCType emits bare TIME (= MySQL TIME(0)) for TimeType(9), so the column stores whole seconds and this reads back as TimeType(0) / 12:00:00. On a real MySQL server both assertions would fail. This wasn't caught because the suite is @DockerTest and isn't part of standard PR CI (the description only reports the JDBCSuite unit run). Once the getJDBCType fix above lands, these assertions become correct as written — just fix the comment premise below.

MaxGekk · 2026-07-14T07:49:11Z

+    // MySQL TIME max precision is 6 (microseconds). TimeType(9) writes as bare TIME (= TIME(6)),
+    // and the nanosecond portion is truncated on read-back.


The premise here is wrong in the same way as the production comment: bare MySQL TIME is TIME(0), not TIME(6). With the getJDBCType fix (emit TIME(6) for out-of-range precision), the accurate statement is:

Suggested change

// MySQL TIME max precision is 6 (microseconds). TimeType(9) writes as bare TIME (= TIME(6)),

// and the nanosecond portion is truncated on read-back.

// MySQL TIME max precision is 6 (microseconds). TimeType(9) writes as TIME(6),

// and the sub-microsecond portion is truncated on read-back.

MaxGekk · 2026-07-14T07:49:11Z

+        // MySQL Connector/J Bug #84308: COLUMN_SIZE=8 and DECIMAL_DIGITS=0 for all TIME columns
+        // regardless of declared precision. The actual precision is injected by
+        // updateExtraColumnMeta via INFORMATION_SCHEMA query.
+        val precision = md.build().getLong("datetimePrecision").toInt


MetadataBuilder.getLong throws NoSuchElementException if datetimePrecision is absent. It's safe today — JdbcUtils.getSchema always calls updateExtraColumnMeta before getCatalystType, and every branch there sets the key for a TIME column — but coupling this read to an unrelated method having run first is fragile. Consider guarding it:

Suggested change

val precision = md.build().getLong("datetimePrecision").toInt

val md0 = md.build()

val precision = if (md0.contains("datetimePrecision")) {

md0.getLong("datetimePrecision").toInt

} else {

TimeType.DEFAULT_PRECISION

}

(Non-blocking.)

Add TimeType support to MySQLDialect: - getCatalystType: maps Types.TIME to TimeType(precision) when timeType.enabled=true and legacyJdbcTimeMappingEnabled=false. Precision is derived from JDBC metadata (scale parameter). - getJDBCType: maps TimeType(p) to TIME(p) DDL for write path. - Integration tests: read round-trip, write round-trip, and legacy escape hatch verification.

…clamping, proactive fixes - getCatalystType: use md.build().getLong("scale") for fractional-second precision (getScale reports DECIMAL_DIGITS, not display width). Accept scale=0 for bare TIME (TIME(0) is valid). - getJDBCType: use TIME(p) for p<=6, bare TIME for out-of-range precisions (e.g. nanosecond TimeType), matching PostgreSQL pattern. - Fix read test: bare TIME column reports scale=0, truncates fractional seconds (value 13:31:24 not 13:31:24.123). - Add precision preservation round-trip test (TimeType(3) -> TIME(3)) - Add nanosecond TimeType(9) write test (MySQL truncates to micros) - Add comment explaining precision source from JDBC metadata

…SION MySQL Connector/J Bug #84308: getPrecision()=8 and getScale()=0 for all TIME columns regardless of declared precision. Override updateExtraColumnMeta to query INFORMATION_SCHEMA.COLUMNS.DATETIME_PRECISION and inject the real precision into the metadata, which getCatalystType then reads.

…comment - getJDBCType: emit TIME(6) instead of bare TIME for out-of-range precisions (bare TIME = TIME(0) loses all fractional seconds) - getCatalystType: guard getLong('datetimePrecision') with contains() check, fallback to DEFAULT_PRECISION if key absent - Fix test comment: TimeType(9) writes as TIME(6), not bare TIME

shrirangmhalgi changed the title ~~Spark 57555 mysql time dialect~~ [SPARK-57555][FOLLOWUP][SQL] Add TimeType support to MySQLDialect Jul 11, 2026

shrirangmhalgi force-pushed the SPARK-57555-mysql-time-dialect branch from 499448d to 75cd5c7 Compare July 11, 2026 14:13

MaxGekk requested changes Jul 14, 2026

View reviewed changes

shrirangmhalgi added 4 commits July 14, 2026 10:31

shrirangmhalgi force-pushed the SPARK-57555-mysql-time-dialect branch from 75cd5c7 to 38cf5f9 Compare July 14, 2026 17:31

shrirangmhalgi mentioned this pull request Jul 14, 2026

[SPARK-57555][FOLLOWUP][SQL] Add TimeType support to MsSqlServerDialect #57253

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-57555][FOLLOWUP][SQL] Add TimeType support to MySQLDialect#57198

[SPARK-57555][FOLLOWUP][SQL] Add TimeType support to MySQLDialect#57198
shrirangmhalgi wants to merge 4 commits into
apache:masterfrom
shrirangmhalgi:SPARK-57555-mysql-time-dialect

shrirangmhalgi commented Jul 11, 2026

Uh oh!

MaxGekk left a comment

Uh oh!

MaxGekk Jul 14, 2026

Uh oh!

MaxGekk Jul 14, 2026

Uh oh!

MaxGekk Jul 14, 2026

Uh oh!

MaxGekk Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// MySQL TIME max precision is 6 (microseconds). TimeType(9) writes as bare TIME (= TIME(6)),
		// and the nanosecond portion is truncated on read-back.

-        val precision = md.build().getLong("datetimePrecision").toInt
+        val md0 = md.build()
+        val precision = if (md0.contains("datetimePrecision")) {
+          md0.getLong("datetimePrecision").toInt
+        } else {
+          TimeType.DEFAULT_PRECISION
+        }

Uh oh!

Conversation

shrirangmhalgi commented Jul 11, 2026

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

MaxGekk left a comment

Choose a reason for hiding this comment

Correctness (2)

Suggestions (1)

Verification

Uh oh!

MaxGekk Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

MaxGekk Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

MaxGekk Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

MaxGekk Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants